Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PP-11232 Add method to find transactions for redaction #2885

Merged
merged 1 commit into from
Sep 19, 2023

Conversation

kbottla
Copy link
Contributor

@kbottla kbottla commented Sep 19, 2023

WHAT

  • Added method to find transactions between a date range for redaction. This is to incrementally find transactions and redact PII from transactions.

- Added method to find transactions between a date range for redaction. This is to incrementally find transactions and redact PII from transactions.
Copy link
Contributor

@jfharden jfharden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic all looks good with nice tests for the limits and date range bounds.

The java looks good to me, but I wont be offended if you want a real java dev to also review :)


import java.time.ZonedDateTime;
import java.time.temporal.ChronoUnit;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need this? I can't see any uses of it in the tests? I was wondering if bringing it in causes some other behaviour?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good spot! it is not not required. I have used it in the test but removed it later. Will remove it in the next PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't cause any behavioural changes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to remember I'm not in ruby land anymore, I'm so used to things in ruby being require'd and then changing the behaviour of everything without ever being referenced directly!

@kbottla kbottla merged commit 69c3839 into master Sep 19, 2023
@kbottla kbottla deleted the pp_11232_add_method_to_find_trxs_to_redact branch September 19, 2023 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants